typescript/ccip-server/src/services/CCTPAttestationService.ts (3)

16-20: Consider making eventNonce optional for consistency.

Since eventNonce is a CCTP v2 field like the others, it should probably be optional too. This would better align with the backward compatibility pattern you've got going here.

Apply this diff:

 interface CCTPMessageEntry {
   attestation: string;
   message: string;
-  eventNonce: string;
+  eventNonce?: string;
   // CCTP v2 only
   cctpVersion?: string;
   status?: Status;
   delayReason?: DelayReason;
 }

---

166-178: Consider adjusting log level for expected pending states.

Looking at this, you're logging all PENDING states as errors, even the ones that are just... well, pending normally. Things like pending_confirmations are expected and temporary - they'll sort themselves out. The serious stuff (like insufficient_fee) deserves the error level, but the rest? Maybe info or warn would fit better.

Based on the past review discussion, I know you want to keep things simple and tune alerts separately, but having the right log level from the start makes life easier down the road.

Here's one way to handle it:

 json.messages.forEach((message) => {
   if (message.attestation === 'PENDING') {
     const errorString = message.delayReason 
       ? `CCTP attestation is pending due to ${message.delayReason}`
       : 'CCTP attestation is still pending';
     switch (message.delayReason) {
       case 'insufficient_fee':
       case 'amount_above_max':
       case 'insufficient_allowance_available':
         PrometheusMetrics.logUnhandledError(this.serviceName);
+        logger.error(context, errorString);
         break;
+      default:
+        logger.info(context, errorString);
+        break;
     }
-    logger.error(context, errorString);
     throw new Error(errorString);
   }
 });

---

166-181: The forEach loop checks all messages but only the first is returned.

You've got this loop checking every message for PENDING states, but then line 181 just grabs the first one and calls it a day. If there's only ever one message per transaction (which the TODO suggests), might be cleaner to just work with json.messages[0] directly. Otherwise, if there could be multiple messages, you'd want to track which one you're actually returning.

If single message is expected:

- json.messages.forEach((message) => {
-   if (message.attestation === 'PENDING') {
+ const message = json.messages[0];
+ if (message.attestation === 'PENDING') {
      // ... existing error handling
-   }
- });
+ }

  // TODO: handle multiple messages in one tx hash
- return [json.messages[0].message, json.messages[0].attestation];
+ return [message.message, message.attestation];

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro